Skip to content

Conversation

@per1234
Copy link
Contributor

@per1234 per1234 commented Jul 30, 2021

Due to the limitations imposed by by using both pull_request_target and issue_comment events to trigger the "Manage PRs" workflow, the PR diff used for the validation is procured via a GitHub API request.

It is necessary to check that the pull request state matches that of the diff, which is achieved via the sha parameter
of the GitHub API request
used to merge. This can not be determined from the github context provided by GitHub Actions to the workflow for either of the trigger events, so the pull request metadata is requested from the GitHub API at the same time as the diff.

This situation requires different handling by the merge-fail job. Fortunately, the two failure causes result in different values from the merge request workflow step's status output.


Demo of updated workflow run through its paces: per1234#16 (comment)

per1234 added 2 commits July 29, 2021 20:10
Previously, the "Manage PRs" workflow made a comment suggesting to resolve the merge conflict after any failure to merge
a pull request. This comment is worded in a way that makes it somewhat applicable to other causes, but still might cause
the submitter to waste time unnecessarily trying to figure out how to merge a nonexistent merge conflict when the failure
had a different cause.

The 405 response is not specific to a failure due to merge conflict, but I believe that all failures due to merge
conflict will result in a 405. This means that the check is not perfect, but will make spurious mentions of merge
conflict resolution less likely at least.

A review is requested from a maintainer any time the merge fails, so they will be able to investigate and provide
assistance if necessary.
Due to the limitations imposed by by using both `pull_request_target` and `issue_comment` events to trigger the
"Manage PRs" workflow, the PR diff used for the validation is procured via a GitHub API request.

It is necessary to check that the pull request state matches that of the diff, which is achieved via the `sha` parameter
of the GitHub API request used to merge. This can not be determined from the `github` context provided by GitHub Actions
to the workflow for either of the trigger events, so the pull request metadata is requested from the GitHub API at the
same time as the diff.

This situation requires different handling by the `merge-fail` job. Fortunately, the two failure causes result in
different values from the merge request workflow step's `status` output.
@per1234 per1234 added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Jul 30, 2021
@per1234 per1234 requested review from silvanocerza and umbynos July 30, 2021 05:33
@github-actions github-actions bot added other and removed topic: code Related to content of the project itself type: enhancement Proposed improvement labels Jul 30, 2021
@github-actions
Copy link
Contributor

Hi @per1234.
Your pull request has been detected as something other than a Library Manager submission.
A maintainer will need to review it before it can be merged.

If you intended to submit a library, please check the instructions and update your pull request if necessary:
https://github.com/arduino/library-registry/blob/main/README.md#instructions

@per1234 per1234 merged commit 2b7e254 into arduino:main Jul 30, 2021
@per1234 per1234 deleted the check-head-sha branch July 30, 2021 12:48
@per1234 per1234 added topic: code Related to content of the project itself type: enhancement Proposed improvement labels Jul 30, 2021
@rsora rsora added the topic: other Something other than a library list request label Sep 22, 2021
@per1234 per1234 removed the other label Oct 20, 2021
@per1234 per1234 self-assigned this Nov 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic: code Related to content of the project itself topic: other Something other than a library list request type: enhancement Proposed improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants